-
Notifications
You must be signed in to change notification settings - Fork 185
[stdlib_math] add arg/argd/argpi
#498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I don't know if there is an issue holding back this PR. I've looked into the files and I think they are fine. print *, argpi(2.0*exp((0.0, PI))), argpi(2.0*exp(cmplx(0.0, -PI))) !! -0.999999940, 0.999999940 by something like: print *, argpi([2.0*exp((0.0, PI)), 2.0*exp(cmplx(0.0, -PI))]) !! -0.999999940, 0.999999940 |
This branch has some conflicts, but most likely a lack of reviewers too. I will try to review it in the next days. |
Thank you for your review @fiolj , I will use |
# Only 1 error: MacOS gfortran 9~11 real(sp) argd failed
Starting argd-cmplx-sp ... (16/26)
179.999985
... argd-cmplx-sp [FAILED]
Message: test_nonzero_scalar Very strange, Should I increase the tolerance value, now |
I can't see what is the problem, and cannot test on that operating system. Being very simple I would try to see what values are giving in MacOs the expressions involved. !> test_argd
program test_argd
implicit none
!> Single precision real numbers
integer, parameter :: sp = selected_real_kind(6)
real(sp), parameter :: tol = epsilon(1.0_sp)
complex(sp) :: z = (-1.0_sp, 0.0_sp)
real(sp) :: angle
real(kind=sp), parameter :: PI = 4.0_sp*atan(1.0_sp)
angle = aimag(log(z))
print *, 'PI =', PI
print *, 'angle =', angle
print *, 'Atan(z) =', atan(aimag(z), real(z))
print *, 'tol =', tol
print *, 'angle=', angle*180/PI, 'diff=', 180._sp - angle*180/PI
!
print *, 'obtained error =', abs(180._sp - 179.999985_sp)
print *, 'original error =', abs(180._sp - 179.999985_sp)*PI/180._sp
end program test_argd
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I have only a major question regarding the tests: do you plan only to test for zero values?
EDIT:
My mistake: you also test for one non-zero value. Question: is it enough?
I apologize for being so late with this comment, but there is a specific reason to use logarithms to calculate the angle? It seems unnecessary to evaluate the logarithm of the real part, if we are only keeping the angle. If I am not missing something may be we can change the code to something like: 1 file changed, 3 insertions(+), 3 deletions(-)
src/stdlib_math.fypp | 6 +++---
modified src/stdlib_math.fypp
@@ -348,7 +348,7 @@ contains
${t1}$, intent(in) :: z
real(${k1}$) :: result
- result = aimag(log(z))
+ result = atan2(aimag(z), real(z))
end function arg_${k1}$
@@ -356,7 +356,7 @@ contains
${t1}$, intent(in) :: z
real(${k1}$) :: result
- result = aimag(log(z))*180.0_${k1}$/PI_${k1}$
+ result = atan2(aimag(z), real(z))*180.0_${k1}$/PI_${k1}$
end function argd_${k1}$
@@ -364,7 +364,7 @@ contains
${t1}$, intent(in) :: z
real(${k1}$) :: result
- result = aimag(log(z))/PI_${k1}$
+ result = atan2(aimag(z), real(z))/PI_${k1}$
end function argpi_${k1}$
#:endfor
|
In fact, it is also possible to use result = merge(0.0_rk, atan2(z%im, z%re), abs(z) == 0.0_rk)
! or
result = merge(0.0_rk, atan2(z%im, z%re), z%im == 0.0_rk .and. z%re == 0.0_rk) print *, log(cmplx(0.0,0.0)) !! (-Infinity,0.0000000E+00) (IFORT compiler) |
There may indeed be an unstable place here: app/example.f90:8:17:
8 | print *, log(cmplx(0.0,0.0))
| 1
Error: Complex argument of LOG at (1) cannot be zero But at runtime, The main reason I use |
I tried to use the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this -- a few changes suggested here -- unrelated to the macos
issue.
update tests.
Three main changes:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks.
Changes look good. Minor issue: using sqrt(epsilon) for tolerance is probably too lax, especially for double precision where typical errors (for example in the values of the tests) are aprox 1.e-14 and tolerance is about 1.e-8. Probably something like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Do you mind adding this addition to the CHANGELOG.md
file?
Fine, I will add later. |
|
Thanks a lot, will merge once the CI has passed. |
arg/argd/argpi
functionsDescription
(idea issue discussions see #489 )
arg
computes the phase angle (radian version) ofcomplex
scalar in the interval(-π,π]
.The angles in
theta
are such thatz = abs(z)*exp((0.0, theta))
.arg
API is:result = [[stdlib_math(module):arg(interface)]](z)
Referring to the Fortran202X, I added

argd/argpi
.(see 19-203r1.txt and 19-204r1.txt)
Prior Art